Fix visual regression suite hang on animation-events test#9
Merged
Conversation
The animation-events test relied on the animation event emitter
(`.once('animating'|'stopped', …)`) which was removed in 222d5173 for
performance. Calling `.once` on the controller now throws, the error
propagates out of the test loop, and `doneTests()` is never called —
Playwright then waits on its done-promise forever.
- examples/index.ts: wrap automation() in try/catch so a thrown test
reports an error and the suite still calls doneTests(). Prevents any
future test from silently hanging CI.
- examples/tests/animation-events.ts: delete. The feature it tested
no longer exists; a rewrite using only waitUntilStopped() was flaky
across capture/compare runs because page.screenshot({ path }) write
timing perturbs in-test animation state. Animations remain covered
by shader-animation.ts.
- visual-regression/certified-snapshots/chromium-ci/animation-events_*:
delete the 8 now-orphaned snapshots.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The SDF text layout cache introduced in 426a22c keyed only on fontFamily, fontSize, letterSpacing, lineHeight, maxHeight, maxWidth, textAlign, and text. Two text nodes that share those values but differ on wordBreak, maxLines, or overflowSuffix would collide on the cache key, and the second node would receive the first node's layout — silently ignoring its own wrap/truncation settings. This fixed 6 visual regression failures: text-wordbreak-1..4, text-max-lines-1, text-overflow-suffix-1. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
With DIRTY_QUAD_BUFFER on, main-scene nodes own permanent slots in quadBuffer and only rewrite when isQuadDirty is true. The RTT pass reset curBufferIdx to 0 and wrote sequentially into the same backing ArrayBuffer, stomping the early main-scene slots. Main scene then uploaded the corrupted buffer to the GPU, so the affected nodes drew RTT vertex data — full-screen background nodes vanished, RTT panel quads jumped to wrong positions, etc. Give RTT its own ArrayBuffer (allocated lazily on first RTT pass) and upload from it in renderRTT. Main-scene slots stay intact; RTT keeps the same sequential rebuild-from-zero semantics it already had. Fixes 6 of 7 RTT visual regression failures (rtt-dimension-1..5, rtt-spritemap-1). rtt-dimension-6 remains failing — that's a separate RTT toggle-off-then-on re-init issue, unrelated to buffer corruption. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
chiefcll
force-pushed
the
fix/visual-regression-hang
branch
from
305dbd2 to
2f151bb
Compare
clearRTTInheritance unconditionally cleared parentHasRenderTexture and rttParent on descendants. That's correct only when the node being detached was the outermost RTT in the chain. For a nested case (rttNode3 > nestedRTTNode1 > rocko4), toggling nestedRTTNode1.rtt off left rocko4 marked as if no RTT ancestor existed — even though rttNode3 was still an active RTT. Re-enabling nestedRTTNode1.rtt later fixed parentHasRenderTexture but rttParent's stale state plus the double-pass during the off->on transition produced visibly broken output for rtt-dimension-6 (rocko4 either disappeared or rendered twice with one copy at parent-local rather than world coords). Look up the nearest still-RTT ancestor and have descendants inherit from it when present, instead of clearing. Re-captured rtt-dimension-6 — the new render places nestedRTTNode1's panel and rocko4 at their geometrically-correct world coordinates, matching the test geometry. The previous certified image came from the original Lightning renderer and showed double-rendered descendants at framebuffer-local coords; that was a bug in the old behavior. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Stage.update walks root.children directly and never invokes root's own
update method (root.updateType is reset to 0 unconditionally). So when
updateViewportBounds set stage.strictBound and flagged root with
RenderBounds, root.createRenderBounds was never called and root's own
strictBound stayed stale. Children copy parent.strictBound in
createRenderBounds, so the staleness propagated all the way down — and
descendants never recomputed their renderState against the new viewport.
For the render-settings test, redText stayed marked InViewport after a
setOptions({appWidth: 550}) shrink, the outOfBounds event never fired,
and the status label kept its old value.
Refresh root.strictBound and root.preloadBound in updateViewportBounds
to match the stage values, so subsequent createRenderBounds copies pick
up the new viewport.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
animation-events_a1-1because the test still called.once('animating'|'stopped', …)on animation controllers — that event emitter API was removed in222d5173("simplify animations, remove emit support for performance"). The thrownTypeErrorpropagated out of the test loop inexamples/index.ts,doneTests()was never invoked, and Playwright's done-promise never resolved.automation()is now wrapped in try/catch, so any future test that throws will log the error and still let the suite calldoneTests()instead of hanging CI.examples/tests/animation-events.tsand its 8 certified snapshots. The feature it exercised no longer exists, and a rewrite using onlywaitUntilStopped()was flaky between capture and compare runs (thepage.screenshot({ path })disk write in capture mode perturbs in-test animation timing). Animations remain covered byshader-animation.ts.Test plan
RUNTIME_ENV=ci pnpm test:visualruns to completion (no hang); 134 pass / 28 pre-existing failures unrelated to this change.🤖 Generated with Claude Code